app/knowledge-base/page.tsx (4)

9-17: Duplicate Document interface definition.

This interface is identical to the one in app/document/page.tsx. Consider importing it from there or extracting to a shared types file to avoid drift and reduce maintenance overhead.

-export interface Document {
-  id: string;
-  fname: string;
-  object_store_url: string;
-  signed_url?: string;
-  file_size?: number;
-  inserted_at?: string;
-  updated_at?: string;
-}
+import { Document } from '@/app/document/page';

---

79-141: Sequential API calls in enrichCollectionWithCache may cause slow loading.

This function is called via Promise.all on line 185-186, but internally it performs sequential API calls within loops (lines 99, 120). For users with many collections, this creates a waterfall of network requests during enrichment.

Consider batching job status requests or fetching all job statuses in a single call if the backend supports it, then matching locally.

---

367-372: Missing dependencies in useEffect may cause stale closures.

The effect depends on apiKey but calls fetchCollections and fetchDocuments which are recreated on each render. Either add them to the dependency array with useCallback, or inline the fetch logic.

+  const fetchCollections = useCallback(async () => {
+    // ... existing implementation
+  }, [apiKey]);
+
+  const fetchDocuments = useCallback(async () => {
+    // ... existing implementation  
+  }, [apiKey]);
+
   useEffect(() => {
     if (apiKey) {
       fetchCollections();
       fetchDocuments();
     }
-  }, [apiKey]);
+  }, [apiKey, fetchCollections, fetchDocuments]);

Alternatively, suppress the lint warning if the current behavior is intentional, but document why.

---

707-711: Use theme color instead of hardcoded hex value.

The delete button uses hardcoded #ef4444 while the project has colors.status.error (#dc2626) defined in the theme. Consider using the theme color for consistency.

                 style={{
                   backgroundColor: 'transparent',
-                  color: '#ef4444',
-                  border: '1px solid `#ef4444`',
+                  color: colors.status.error,
+                  border: `1px solid ${colors.status.error}`,
                 }}

app/knowledge-base/page.tsx (5)

9-17: Consider consolidating the duplicate Document interface.

This Document interface is identical to the one exported from app/document/page.tsx. Duplicating type definitions can lead to drift over time. Consider importing from a shared types file or directly from the document page.

-export interface Document {
-  id: string;
-  fname: string;
-  object_store_url: string;
-  signed_url?: string;
-  file_size?: number;
-  inserted_at?: string;
-  updated_at?: string;
-}
+import { Document } from '../document/page';

Alternatively, create a shared types module (e.g., app/types/index.ts) to house common interfaces.

---

94-119: Sequential API calls in loop may degrade performance.

When the cache has many entries without collection_id, this loop makes sequential fetchJobStatus calls for each one. With a large cache, this can significantly slow down the initial load.

Consider batching these lookups or limiting the number of iterations to avoid excessive API calls during enrichment.

---

275-333: Polling iterations may overlap if API calls take longer than 3 seconds.

The setInterval callback is async, but setInterval doesn't wait for the previous iteration to complete. If network requests are slow, multiple polling cycles could run concurrently, causing redundant API calls and potential race conditions when updating state.

Consider adding a guard to skip the iteration if the previous one is still in progress:

Proposed fix

+  const isPollingRef = useRef(false);
+
   pollingRef.current = setInterval(async () => {
+    if (isPollingRef.current) return;
+    isPollingRef.current = true;
+    try {
       const currentApiKey = apiKeyRef.current;
       // ... existing logic ...
+    } finally {
+      isPollingRef.current = false;
+    }
   }, 3000);

---

496-501: useEffect has missing dependencies that may cause linter warnings.

fetchCollections and fetchDocuments are called inside this effect but are not listed as dependencies. Since these functions are recreated on each render, adding them would cause infinite loops. Consider wrapping them with useCallback to stabilize their references, or add an ESLint disable comment if intentional.

+  // eslint-disable-next-line react-hooks/exhaustive-deps
   useEffect(() => {
     if (apiKey) {
       fetchCollections();
       fetchDocuments();
     }
   }, [apiKey]);

---

1135-1141: Consider adding sandbox attribute to the iframe for defense-in-depth.

The iframe loads document content from signed URLs. Adding a sandbox attribute restricts the embedded content's capabilities, providing an additional security layer.

   <iframe
     key={previewDoc.id}
     src={previewDoc.signed_url}
     title={previewDoc.fname}
     className="w-full h-full"
-    style={{ border: 'none' }}
+    style={{ border: 'none' }}
+    sandbox="allow-same-origin allow-scripts"
   />

Note: Adjust sandbox permissions based on what the previewed documents require (e.g., allow-scripts only if needed for PDF viewers).

app/knowledge-base/page.tsx (3)

9-17: Document interface is duplicated from app/document/page.tsx.

Both files export an identical Document interface. Consider extracting it into a shared types file (e.g., app/types.ts) to avoid drift.

---

86-148: enrichCollectionWithCache fires N×M network requests on every fetch.

Each collection triggers up to one fetchJobStatus call per uncached job entry in localStorage (lines 100-121), plus another if jobId is found but status is missing (lines 125-139). With many collections and cache entries, this fans out into a large number of sequential-per-collection, parallel-across-collections API calls on every fetchCollections invocation. Consider short-circuiting enrichment for collections that already have a terminal status cached, or batch-fetching job statuses.

---

984-1022: Modals lack Escape-key handling and focus trapping.

The four modals (confirm create, confirm delete, document picker, document preview) don't listen for the Escape key or trap focus. The UploadDocumentModal in app/document/page.tsx already implements Escape handling — consider applying the same pattern here for consistency and accessibility.

app/knowledge-base/page.tsx (5)

7-7: Importing types from another page component creates tight coupling.

APIKey and STORAGE_KEY are implementation details of the keystore page. If that page is refactored, this import breaks. Consider extracting shared types/constants into a dedicated module (e.g., app/lib/keystore.ts).

---

541-546: Missing dependencies in effect — fetchCollections and fetchDocuments are not stable.

fetchCollections and fetchDocuments are recreated every render (not wrapped in useCallback), so omitting them from the dependency array silences the exhaustive-deps lint rule but means the effect captures a stale closure on the initial render. This works today only because both functions read apiKey from component scope and the effect re-runs when apiKey changes.

Consider wrapping both in useCallback (with [apiKey] deps) or using refs consistently as you've done for fetchCollectionsRef.

---

421-421: Hardcoded provider: 'openai'.

This value is not configurable by the user. If additional providers are supported in the future, this will need to change. Consider making it a form field or a configuration constant.

---

30-1211: Consider splitting this ~1200-line component into smaller pieces.

This single component handles data fetching, polling, caching, optimistic updates, and renders multiple modals + list/detail views. Extracting logical units (e.g., CollectionList, CreateCollectionForm, DocumentPickerModal, custom hooks for polling and collection CRUD) would improve readability and testability.

---

56-67: localStorage cache is never pruned.

Entries in collection_job_cache are added on create but never removed on delete or after jobs complete. Over time this grows unbounded. Consider cleaning up entries when a collection is deleted or when the job reaches a terminal state.

app/knowledge-base/page.tsx (5)

7-7: Consider extracting APIKey and STORAGE_KEY into a shared module.

Importing from another page's module (../keystore/page) creates a tight coupling between pages. If the keystore page is refactored, this import breaks. Move these shared types/constants to a dedicated file (e.g., app/lib/auth.ts).

---

696-707: fetchCollections / fetchDocuments missing from dependency array; fetchCollections is not memoized.

The effect at line 696 calls fetchCollections() and fetchDocuments() but omits them from the [apiKey] dependency array — this will trigger the react-hooks/exhaustive-deps lint rule. Meanwhile, line 707 lists fetchCollections as a dependency, but since it's not wrapped in useCallback, the ref-sync effect fires on every render.

Consider wrapping fetchCollections and fetchDocuments in useCallback (with [apiKey] as a dependency) so the effects have stable references.

---

546-668: Extract the repeated restore-on-error logic in handleConfirmDelete.

The pattern of restoring originalCollection into both setCollections and setSelectedCollection is repeated five times in this function. Extract it into a helper to reduce duplication and the risk of inconsistent updates.

Example

+  const restoreCollection = (collectionId: string, original: Collection | undefined) => {
+    if (!original) return;
+    setCollections((prev) =>
+      prev.map((c) => (c.id === collectionId ? original : c))
+    );
+    setSelectedCollection((prev) =>
+      prev?.id === collectionId ? original : prev
+    );
+  };

Then replace each occurrence with restoreCollection(collectionId, originalCollection).

---

30-50: Consider splitting this ~1400-line component into smaller pieces.

This single component handles data fetching, polling, caching, form state, and renders multiple modals. Extracting custom hooks (e.g., useCollections, useJobPolling) and modal sub-components would improve readability and testability. Not blocking, but worth planning for.

---

487-492: Hardcoded provider: 'openai'.

The provider is hardcoded. If other providers are supported (or planned), this should be configurable — e.g., via an environment variable or a form field. Fine for an MVP, but worth tracking.